Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🌐 Spherical coordinates #1008

Merged
merged 13 commits into from
Sep 22, 2021
Merged

🌐 Spherical coordinates #1008

merged 13 commits into from
Sep 22, 2021

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Sep 4, 2021 •
edited
Loading

🎉 New feature

Closes #981

Requires

Summary

Add support for spherical coordinates (latitude / longitude). Users are able to set the lat / lon for the world origin, and query the lat / lon for other entities with respect to that.

  • Set from SDF, see example world
  • See on component inspector
  • Plot world origin coordinates
  • Change world origin coordinates from plugins
  • Change world origin coordinates from a transport service

spherical_coors

TODO

  • Tutorial

Under consideration:

  • Support spawning entities at a given spherical location through the create service
  • Move entities to a given spherical coordinate through the set pose service

I'll open this PR for review when the above have been sorted out.

Test it

Try changing it through the SDF, the component inspector, and the service.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

@chapulina
Beginning of spherical coordinates
32c4c50 
Signed-off-by: Louise Poubel <[email protected]>
@chapulina
display and plot
974c115 
Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Service and test
67b71a6 
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina added close the gap Features from Gazebo-classic needs upstream release Blocked by a release of an upstream library labels Sep 4, 2021
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Sep 4, 2021
@chapulina chapulina mentioned this pull request Sep 8, 2021
Merged
8 tasks
@chapulina chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2021
@chapulina chapulina mentioned this pull request Sep 8, 2021
Merged
8 tasks
@chapulina chapulina marked this pull request as ready for review September 15, 2021 04:21
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Sep 17, 2021
@codecov
Copy link

codecov bot commented Sep 18, 2021 •
edited
Loading

Codecov Report

Merging #1008 (1fa8bae) into main (4be711c) will decrease coverage by 0.09%.
The diff coverage is 62.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1008      +/-   ##
==========================================
- Coverage   63.47%   63.37%   -0.10%     
==========================================
  Files         241      242       +1     
  Lines       19577    19715     +138     
==========================================
+ Hits        12426    12495      +69     
- Misses       7151     7220      +69
Impacted Files Coverage Δ
include/ignition/gazebo/Util.hh 100.00% <ø> (ø)
include/ignition/gazebo/World.hh 100.00% <ø> (ø)
.../plugins/component_inspector/ComponentInspector.cc 6.22% <0.00%> (-0.52%) ⬇️
.../plugins/component_inspector/ComponentInspector.hh 28.57% <ø> (ø)
src/SdfEntityCreator.cc 84.14% <33.33%> (-0.40%) ⬇️
...nclude/ignition/gazebo/components/Serialization.hh 94.11% <60.00%> (-5.89%) ⬇️
src/Util.cc 93.10% <80.00%> (-0.60%) ⬇️
src/systems/user_commands/UserCommands.cc 60.88% <81.05%> (+2.93%) ⬆️
...ignition/gazebo/components/SphericalCoordinates.hh 100.00% <100.00%> (ø)
src/LevelManager.cc 89.14% <100.00%> (+0.11%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4be711c...1fa8bae. Read the comment docs.

@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Sep 18, 2021
@chapulina
help Windows
8936911 
Signed-off-by: Louise Poubel <[email protected]>
caguero
caguero reviewed Sep 21, 2021
View reviewed changes
Copy link
Contributor

@caguero caguero left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything worked as expected. I just found a tiny typo in the tutorial. I have just one comment:

Let's say I follow the tutorial and spawn an entity. If I now set the world origin's spherical coordinates with the GUI or the service to a different value, shouldn't all entities be moved as a side effect?

tutorials/spherical_coordinates.md Outdated Show resolved Hide resolved
tutorials/spherical_coordinates.md Show resolved Hide resolved
@chapulina
tutorial clarifications
76495a7 
Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

Let's say I follow the tutorial and spawn an entity. If I now set the world origin's spherical coordinates with the GUI or the service to a different value, shouldn't all entities be moved as a side effect?

I added a note about this to the tutorial in 76495a7. It would be tough to move the entire world, and I'm not sure this is something that the user may always want. I think in the future, if there's need for it, we could add an option that enables this. For now, changing the world origin only affects subsequent commands, like spawns and moves.

@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Sep 22, 2021
@chapulina chapulina merged commit 2b141ac into main Sep 22, 2021
@chapulina chapulina deleted the chapulina/6/spherical_coords branch September 22, 2021 05:29
WilliamLewww pushed a commit to WilliamLewww/ign-gazebo that referenced this pull request Dec 7, 2021
@chapulina @WilliamLewww
🌐 Spherical coordinates (gazebosim#1008)
8c7a710 
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: William Lew <[email protected]>
@chapulina chapulina mentioned this pull request Dec 21, 2021
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caguero caguero caguero approved these changes

Assignees
No one assigned
Labels
beta Targeting beta release of upcoming collection close the gap Features from Gazebo-classic 🏯 fortress Ignition Fortress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chapulina @caguero